Skip to content

fix: Add security header stripping in vcl_recv + test#126

Open
furan917 wants to merge 1 commit into
elgentos:masterfrom
furan917:fix/header-injection-prevention
Open

fix: Add security header stripping in vcl_recv + test#126
furan917 wants to merge 1 commit into
elgentos:masterfrom
furan917:fix/header-injection-prevention

Conversation

@furan917

@furan917 furan917 commented Apr 2, 2026

Copy link
Copy Markdown

Varnish VCL forwarding some known abusable headers to the Magento backend.

  • X-Original-URL / X-Rewrite-URL - can be used to bypass access controls in Symfony/Laravel/Magento by overriding the URL the application sees
  • X-Forwarded-Host / X-Forwarded-Server - can be abused for host header injection, leading to cache poisoning or password reset link hijacking
  • Proxy - mitigates HTTPoxy (CVE-2016-5387), where this header maps to HTTP_PROXY in PHP-FPM environments and can redirect outbound requests through an attacker-controlled proxy.

@furan917

furan917 commented Apr 3, 2026

Copy link
Copy Markdown
Author

That error was more annoying to find the fix for - caused by fresh pulling in v9 not 7.x
I think the easiest fix there is updating the cache header vtc to have a b2 cond 2 and a barrier b2 sync after the txresp and again at the end of the file to ensure the s1 stop isn't called too soon. That way there should be no race condition that calls stop while somethings still in the process queue.

(Or pin to 7.x I suppose is an option)

@rhoerr

rhoerr commented Apr 27, 2026

Copy link
Copy Markdown

@toonvd @peterjaap Any chance you can review this and #127 ? Thanks!

@toonvd

toonvd commented May 12, 2026

Copy link
Copy Markdown
Collaborator

Should be solved at offload or webserver level imho. Magento (and a lot of hostings) use Varnish for caching, not for security.

@furan917

furan917 commented May 12, 2026

Copy link
Copy Markdown
Author

I don't know anyone who uses varnish as their security layer to be fair, but its the swiss cheese model/defensive posturing. Not everyone who uses this will have something in front, or something in-between (that strips them) to handle this, its just a small addition to ensure this VCL isnt one of those holes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants